Expose load_dataset and load_multiview_dataset from top-level movement module (closes #361)#863
Conversation
…t module Closes neuroinformatics-unit#361 Users can now import loading functions directly from the movement package instead of the movement.io submodule: import movement as mvt ds = mvt.load_dataset('file.h5', source_software='DeepLabCut', fps=30) - Re-export load_dataset and load_multiview_dataset in movement/__init__.py - Add __all__ to document the public API - Add tests/test_unit/test_top_level_imports.py to verify the exports
|
There was a problem hiding this comment.
Pull request overview
This PR aims to make the dataset loading API more discoverable by re-exporting the core loader functions (load_dataset, load_multiview_dataset) from the top-level movement package interface.
Changes:
- Re-exports
load_datasetandload_multiview_datasetinmovement/__init__.py. - Defines
__all__inmovement/__init__.pyto explicitly declare the intended public surface. - Adds a unit test module to verify top-level importability and object identity of the re-exports.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
movement/__init__.py |
Adds top-level re-exports and __all__ entries for the loader functions. |
tests/test_unit/test_top_level_imports.py |
Adds unit tests validating top-level import behavior and identity with the underlying functions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -1,5 +1,6 @@ | |||
| from importlib.metadata import PackageNotFoundError, version | |||
|
|
|||
| from movement.io.load import load_dataset, load_multiview_dataset | |||
There was a problem hiding this comment.
Re-exporting from movement.io.load means importing movement does not trigger movement.io.__init__ (which imports load_poses/load_bboxes to populate _LOADER_REGISTRY). As a result, movement.load_dataset(...) will raise ValueError("Unsupported source software") because the registry is empty. Re-export from movement.io instead, or ensure loader modules are imported (e.g., lazy-import load_poses/load_bboxes inside load_dataset when the registry is empty).
| from movement.io.load import load_dataset, load_multiview_dataset | |
| from movement.io import load_dataset, load_multiview_dataset |
| def test_load_dataset_importable_from_top_level(): | ||
| """Test that load_dataset can be imported from movement directly.""" | ||
| from movement import load_dataset | ||
|
|
||
| assert callable(load_dataset) | ||
|
|
||
|
|
||
| def test_load_multiview_dataset_importable_from_top_level(): | ||
| """Test that load_multiview_dataset can be imported from movement.""" | ||
| from movement import load_multiview_dataset | ||
|
|
||
| assert callable(load_multiview_dataset) | ||
|
|
||
|
|
||
| def test_top_level_load_dataset_is_same_as_io(): | ||
| """Top-level load_dataset is the same object as movement.io.load's.""" | ||
| from movement import load_dataset | ||
| from movement.io.load import load_dataset as io_load_dataset | ||
|
|
||
| assert load_dataset is io_load_dataset |
There was a problem hiding this comment.
These tests only check importability/object identity, but they don’t verify that the new public API actually works when used as intended (import movement as mvt; mvt.load_dataset(...)) without separately importing movement.io to register loaders. Add a test that calls the top-level function (e.g., by patching movement.io.load._LOADER_REGISTRY and asserting delegation) so this regression is caught.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #863 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 38 38
Lines 2284 2286 +2
=========================================
+ Hits 2284 2286 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Expose
load_datasetandload_multiview_datasetfrom top-levelmovementmoduleCloses #361
Description
What is this PR?
Why is this PR needed?
Previously, users had to import the core loading functions from the submodule path
movement.io.load, which is an implementation detail rather than a stable public interface. Exposing these functions at the top level makes the API more discoverable and ergonomic, consistent with how users typically interact with a package:What does this PR do?
load_datasetandload_multiview_datasetinmovement/__init__.pyby importing them frommovement.io.load__all__tomovement/__init__.pyto formally document the public API surfacetests/test_unit/test_top_level_imports.pywith 4 tests that verify:load_datasetis importable directly frommovementload_multiview_datasetis importable directly frommovementload_datasetis the exact same object asmovement.io.load.load_dataset(no wrapping or copying)load_multiview_datasetis the exact same object asmovement.io.load.load_multiview_datasetReferences
Closes #361
How has this PR been tested?
A new test file
tests/test_unit/test_top_level_imports.pywas added. It tests both importability and object identity (i.e., the re-exported functions are the same objects as their source, not copies or wrappers), ensuring no indirection was introduced. No existing tests were modified, and no existing functionality was changed — this PR only adds re-exports.Is this a breaking change?
No. This PR is purely additive. The functions remain available at their original
movement.io.loadpath; this PR only adds an additional, more convenient import path.Does this PR require an update to the documentation?
The
__all__list inmovement/__init__.pysignals to documentation generators thatload_datasetandload_multiview_datasetare part of the public API. Ifmovement's top-level module is listed indocs/make_api.pyunderPACKAGE_MODULES, these functions will appear automatically in the API reference. No manual documentation changes are required beyond verifying the API docs build correctly.Checklist